fix: address audit bug regressions by elkaix · Pull Request #29 · Pythoughts-labs/pythinker-code

elkaix · 2026-05-31T00:47:41Z

Summary

make StrReplaceFile reject missing, empty, no-op, and ambiguous replacements before writing
harden background runtime updates, compaction end signaling, memory dedup/recall, and shell markup escaping
remove completed task planning docs and add regression coverage

Verification

git diff --check
uv run pytest tests/tools/test_str_replace_file.py tests/core/test_compaction_restore.py tests/core/test_memory_phase_bcd.py tests/ui_and_conv/test_shell_export_import_commands.py tests/tools/test_tool_descriptions.py

Summary by CodeRabbit

Release Notes

New Features
- Enhanced Unicode support in memory search functionality for improved accuracy.
Bug Fixes
- File replacement tool now strictly validates replacements and rejects ambiguous matches without explicit all-match flag.
- Error messages properly escaped to prevent formatting display issues.
- Compaction process now reliably closes streams even when errors occur.
- Session picker header adjusted for improved display when no sessions exist.

coderabbitai · 2026-05-31T00:47:52Z

📝 Walkthrough

Walkthrough

This PR tightens StrReplaceFile validation to reject ambiguous replacements, refactors background-task runtime updates via callbacks, enhances memory retrieval with Unicode support and label-based search, ensures compaction wire events emit even on failure, escapes dynamic text in Rich markup across UI commands, and optimizes MCP tool loading with early returns.

Changes

StrReplaceFile validation tightening

Layer / File(s)	Summary
StrReplaceFile validation and batch edits `src/pythinker_code/tools/file/replace.py`	Implementation adds pre-write validation: empty edits, empty `old` strings, no-op edits (`old == new`), missing matches, and ambiguous multiple-match detection unless `replace_all=true`. Per-edit replacement count tracking replaces prior recomputation.
StrReplaceFile docs and test coverage `CHANGELOG.md`, `src/pythinker_code/tools/file/replace.md`, `tests/tools/test_str_replace_file.py`, `tests/tools/test_tool_descriptions.py`	Tool documentation and changelog describe exact-match requirement; comprehensive test suite covers multi-edit batches, chained edits, missing/ambiguous matches, empty strings, and empty lists.

Background task runtime update refactoring

Layer / File(s)	Summary
BackgroundTaskStore.update_runtime callback pattern `src/pythinker_code/background/store.py`	New `update_runtime(task_id, update)` method accepts a callable that mutates runtime and returns a persistence flag, acquiring the per-task lock for atomic read-modify-write cycles.
BackgroundTaskManager post-launch worker marking `src/pythinker_code/background/manager.py`	Post-launch bookkeeping refactored to use `update_runtime` callback that atomically transitions task to `starting`, records `worker_pid`, and updates timestamp only when preconditions are met.
Worker shutdown and final status computation `src/pythinker_code/background/worker.py`	`finish_runtime` closure centralizes final status/failure determination from returncode and interrupt flags (output limit, timeout, kill request), called via `update_runtime` after process completes.

Memory retrieval and inbox consolidation

Layer / File(s)	Summary
Inbox candidate digest consolidation `src/pythinker_code/memory/consolidation.py`	New `_memory_entry_hash` helper computes digests consistently; consolidation stages both on-disk and computed memory-entry hashes; `FileExistsError` on `O_EXCL` write is caught and handled as already-staged rather than crashing.
LexicalRetriever Unicode and label-based search `src/pythinker_code/memory/retriever.py`	`_tokenize` adds Unicode `NFKC` normalization and `casefold()`; `_TOKEN_RE` extracts Unicode word runs treating underscores as separators; `RecallQuery.labels` included in BM25 input terms; label-overlap boost removed.
Memory retrieval and consolidation tests `tests/core/test_memory_phase_bcd.py`	Test helper `_ranked` constructs `RankedBlock` fixtures; new tests verify Unicode term matching, labels-as-terms search, scratch-note skipping, and corrupt duplicate file tolerance.

Compaction context rebuild and wire event guarantees

Layer / File(s)	Summary
Compaction inline context rebuild and hooks `src/pythinker_code/soul/pythinkersoul.py`	Post-compaction context now explicitly rebuilt inline: system prompt + checkpoint + compaction messages + restored messages + optional root task snapshot, with sequential token estimation; hooks triggered; history rebuild notified to injection providers; `CompactionEnd` emitted in `finally` block.
Compaction failure wire event test `tests/core/test_compaction_restore.py`	New test verifies `CompactionBegin` and `CompactionEnd` both emitted exactly once when compaction run fails; telemetry recorded with `success=False`.

UI markup safety across shell commands

Layer / File(s)	Summary
Main shell and command error escaping `src/pythinker_code/ui/shell/__init__.py`	Wraps all dynamic exception/error rendering in `rich.markup.escape`: ignored-input args, shell-command failures, slash-command errors, `LLMNotSupported`, and `ChatProviderError` messages.
Export, import, setup, browser, and usage command escaping `src/pythinker_code/ui/shell/export_import.py`, `src/pythinker_code/ui/shell/setup.py`, `src/pythinker_code/ui/shell/task_browser.py`, `src/pythinker_code/ui/shell/usage.py`	Escapes user-facing paths and error messages in export/import, setup platform/model names, task browser IDs, and usage errors.
Export with bracketed paths test `tests/ui_and_conv/test_shell_export_import_commands.py`	New test verifies export command correctly handles and displays file paths containing bracket characters without markup formatting interference.

UI edge cases and optimizations

Layer / File(s)	Summary
Session picker zero-sessions and MCP optimization `src/pythinker_code/ui/shell/session_picker.py`, `src/pythinker_code/soul/toolset.py`	Session picker header simplified for zero-sessions case; `load_mcp_tools` imports `toast` lazily inside helper; early return when no MCP servers are pending skips `_connect` scheduling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

TechMatrix-labs/pythinker-code#13: Background task runtime refactoring directly extends runtime update reliability improvements from that PR.

Suggested labels

bug

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description provides a clear summary of changes and verification steps, but omits required sections from the template: no related issue link, incomplete checklist, and missing documentation update confirmation.	Add 'Resolve #(issue_number)' for the related issue, complete all checklist items with explicit confirmations, and confirm whether gen-docs was run.
Docstring Coverage	⚠️ Warning	Docstring coverage is 38.46% which is insufficient. The required threshold is 70.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title uses conventional commits format (fix: scope) and accurately summarizes the PR's core objective of addressing audit-related bug regressions across multiple components.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/audit-bugs

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/pythinker_code/ui/shell/__init__.py (1)
1264-1264: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Escape ChatProviderError messages in "Server:" lines.

Lines 1264, 1270, 1275, and 1280 render exception objects without escaping, allowing markup injection if error messages contain bracket characters (e.g., from URLs, JSON responses, or file paths).
🔒 Proposed fix
-                        f"[dim]Server: {e}[/dim]"
+                        f"[dim]Server: {escape(str(e))}[/dim]"
Apply this pattern to all four occurrences (lines 1264, 1270, 1275, 1280).
Also applies to: 1270-1270, 1275-1275, 1280-1280
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pythinker_code/ui/shell/__init__.py` at line 1264, The "Server:" lines
interpolate ChatProviderError exception messages directly (pattern:
f"[dim]Server: {e}[/dim]") which permits markup injection; update each
occurrence to escape the exception text before rendering by importing
rich.markup.escape (or equivalent) and replacing {e} with escape(str(e)) so the
f-strings become f"[dim]Server: {escape(str(e))}[/dim]" in the code paths where
ChatProviderError is handled; apply this change to all four occurrences that
currently use the unescaped pattern.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/pythinker_code/ui/shell/__init__.py`:
- Line 1264: The "Server:" lines interpolate ChatProviderError exception
messages directly (pattern: f"[dim]Server: {e}[/dim]") which permits markup
injection; update each occurrence to escape the exception text before rendering
by importing rich.markup.escape (or equivalent) and replacing {e} with
escape(str(e)) so the f-strings become f"[dim]Server: {escape(str(e))}[/dim]" in
the code paths where ChatProviderError is handled; apply this change to all four
occurrences that currently use the unescaped pattern.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 90406527-a4c8-4387-97ed-f1b072170a43

📥 Commits

Reviewing files that changed from the base of the PR and between b321a07 and e2091d0.

📒 Files selected for processing (25)

CHANGELOG.md
src/pythinker_code/background/manager.py
src/pythinker_code/background/store.py
src/pythinker_code/background/worker.py
src/pythinker_code/memory/consolidation.py
src/pythinker_code/memory/retriever.py
src/pythinker_code/soul/pythinkersoul.py
src/pythinker_code/soul/toolset.py
src/pythinker_code/tools/file/replace.md
src/pythinker_code/tools/file/replace.py
src/pythinker_code/ui/shell/__init__.py
src/pythinker_code/ui/shell/export_import.py
src/pythinker_code/ui/shell/session_picker.py
src/pythinker_code/ui/shell/setup.py
src/pythinker_code/ui/shell/task_browser.py
src/pythinker_code/ui/shell/usage.py
tasks/agent-behavior-findings.md
tasks/port-bk-box-agent-features.md
tasks/tui-enhancement-plan.md
tasks/tui-text-color-standardization.md
tests/core/test_compaction_restore.py
tests/core/test_memory_phase_bcd.py
tests/tools/test_str_replace_file.py
tests/tools/test_tool_descriptions.py
tests/ui_and_conv/test_shell_export_import_commands.py

💤 Files with no reviewable changes (4)

tasks/tui-enhancement-plan.md
tasks/tui-text-color-standardization.md
tasks/agent-behavior-findings.md
tasks/port-bk-box-agent-features.md

fix: address audit bug regressions

e2091d0

coderabbitai Bot reviewed May 31, 2026

View reviewed changes

Merge branch 'main' into fix/audit-bugs

771b80b

elkaix merged commit e8cb860 into main May 31, 2026
19 checks passed

elkaix deleted the fix/audit-bugs branch May 31, 2026 01:29

coderabbitai Bot mentioned this pull request May 31, 2026

fix(update): startup readiness and reliability cleanups #30

Merged

4 tasks

elkaix mentioned this pull request May 31, 2026

chore(release): prepare 0.27.0 #34

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: address audit bug regressions#29

fix: address audit bug regressions#29
elkaix merged 2 commits into
mainfrom
fix/audit-bugs

elkaix commented May 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 31, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

elkaix commented May 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

elkaix commented May 31, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 31, 2026 •

edited

Loading